resource/aws_route53_record: Switch allow_overwrite default from true to false#7734
Merged
Merged
Conversation
bflad
added
breaking-change
ghost
added
size/S
… to false References: * #3895 * #2926 Previously, the `aws_route53_record` resource did not follow standard Terraform conventions of requiring existing infrastructure to be imported into Terraform's state for management, which meant operators could unexpectedly affect that existing infrastructure. In version 1.10.0, we introduced the `allow_overwrite` argument so operators could opt into the upcoming import requirement and force the Terraform resource during resource creation to error if it attempted to create a Route53 record that previously existed. Here we make the breaking change to switch the default resource behavior to error on creation for existing records. Operators can opt out of the new behavior by enabling the flag, but it is marked as deprecated for removal in the next major version and will display the deprecation warning when used to signal that workflows should be adjusted if necessary. Output from acceptance testing: ``` --- PASS: TestAccAWSRoute53Record_Alias_Elb (319.83s) --- PASS: TestAccAWSRoute53Record_Alias_S3 (123.47s) --- PASS: TestAccAWSRoute53Record_Alias_Uppercase (184.67s) --- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (450.17s) --- PASS: TestAccAWSRoute53Record_AliasChange (157.29s) --- PASS: TestAccAWSRoute53Record_allowOverwrite (365.48s) --- PASS: TestAccAWSRoute53Record_basic (130.53s) --- PASS: TestAccAWSRoute53Record_basic_fqdn (146.30s) --- PASS: TestAccAWSRoute53Record_caaSupport (177.87s) --- PASS: TestAccAWSRoute53Record_disappears (107.58s) --- PASS: TestAccAWSRoute53Record_disappears_MultipleRecords (247.77s) --- PASS: TestAccAWSRoute53Record_empty (115.48s) --- PASS: TestAccAWSRoute53Record_failover (204.75s) --- PASS: TestAccAWSRoute53Record_generatesSuffix (180.48s) --- PASS: TestAccAWSRoute53Record_geolocation_basic (196.58s) --- PASS: TestAccAWSRoute53Record_importBasic (174.28s) --- PASS: TestAccAWSRoute53Record_importUnderscored (114.40s) --- PASS: TestAccAWSRoute53Record_latency_basic (173.99s) --- PASS: TestAccAWSRoute53Record_longTXTrecord (114.97s) --- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (197.09s) --- PASS: TestAccAWSRoute53Record_SetIdentifierChange (206.47s) --- PASS: TestAccAWSRoute53Record_spfSupport (152.57s) --- PASS: TestAccAWSRoute53Record_txtSupport (170.00s) --- PASS: TestAccAWSRoute53Record_TypeChange (220.81s) --- PASS: TestAccAWSRoute53Record_weighted_alias (278.61s) --- PASS: TestAccAWSRoute53Record_weighted_basic (112.68s) --- PASS: TestAccAWSRoute53Record_wildcard (216.04s) ```
bflad
force-pushed
the
td-aws_route53_record-allow_overwrite-default
branch
from
e60b809 to
a8c6bdf
Compare
paddycarver
reviewed
Feb 26, 2019
|
|
||
| ### allow_overwrite Default Value Change | ||
|
|
||
| The resource now requires existing Route 53 Records to be imported into the Terraform state for management unless the `allow_overwrite` argument is enabled. The `allow_overwrite` flag is considered deprecated for removal in the next major version of the Terraform AWS Provider (version 3.0.0). |
Contributor
There was a problem hiding this comment.
Suggestion: don't name a specific version, just say "a future version". Then if you forget for 3.0.0, you didn't break a promise.
paddycarver
approved these changes
Feb 26, 2019
…nt as future major version, not necessarily next
bflad
added a commit
that referenced
this pull request
Feb 26, 2019
wking
added a commit
to wking/openshift-installer
that referenced
this pull request
Mar 26, 2019
This picks up a bunch of upstream improvements [1] including [2] to address [3]. Generated with a manual Gopkg.toml edit followed by: $ cd terraform/exec/plugins $ dep ensure with: $ dep version dep: version : v0.5.1 build date : 2019-03-20 git hash : faa61893 go version : go1.10.3 go compiler : gc platform : linux/amd64 features : ImportDuringSolve=false The aws-iam-authenticator pin gets us closer to [4], although (1) I don't know why dep isn't figuring that out for itself and (2) they got rid of their 0.3.1 tag [5]? Anyway, this avoids: $ hack/build.sh ... # github.com/openshift/installer/pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws/data_source_aws_eks_cluster_auth.go:35:38: too many arguments in call to token.NewGenerator have (bool) want () ... The AWS SDK pin gets us closer to [6] and avoids: $ hack/build.sh ... # github.com/openshift/installer/pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws/structure.go:4801:7: spec.ServiceNames undefined (type *appmesh.VirtualRouterSpec has no field or method ServiceNames) ... The = prefixes in Gopkg.toml settle things down, because [7]: Note: When you specify a version without an operator, dep automatically uses the ^ operator by default. dep ensure will interpret the given version as the min-boundary of a range... [1]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/CHANGELOG.md [2]: hashicorp/terraform-provider-aws#7734 (v2.0.0) [3]: https://bugzilla.redhat.com/show_bug.cgi?id=1659970 [4]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/go.mod#L27 [5]: https://github.com/kubernetes-sigs/aws-iam-authenticator/tags [6]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/go.mod#L7 [7]: https://golang.github.io/dep/docs/Gopkg.toml.html#version-rules
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
ghost
locked and limited conversation to collaborators
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3895
Reference: #2926
Previously, the
aws_route53_recordresource did not follow standard Terraform conventions of requiring existing infrastructure to be imported into Terraform's state for management, which meant operators could unexpectedly affect that existing infrastructure. In version 1.10.0, we introduced theallow_overwriteargument so operators could opt into the upcoming import requirement and force the Terraform resource during resource creation to error if it attempted to create a Route53 record that previously existed.Here we make the breaking change to switch the default resource behavior to error on creation for existing records. Operators can opt out of the new behavior by setting
allow_overwrite = true, but it is marked as deprecated for removal in the next major version and will display the deprecation warning when used to signal that workflows should be adjusted if necessary.Output from acceptance testing: